Improve action log step summary: 2-line tool previews and nicer agent messages#24558
Improve action log step summary: 2-line tool previews and nicer agent messages#24558
Conversation
…r agent messages - Add formatResultPreview() helper that shows first 2 lines of tool output using tree-branch characters (├/└) for visual hierarchy - Update generateCopilotCliStyleSummary() to use new helper for all tool types - Update generatePlainTextSummary() to use new helper for all tool types - Change agent message prefix from "Agent: " (per-line) to "◆ " (first line) with " " indent for continuation lines, for a cleaner visual style - Count multi-line resultPreview accurately in conversationLineCount - Export formatResultPreview from module for potential reuse - Update all tests to match new formatting Agent-Logs-Url: https://github.com/github/gh-aw/sessions/e919a297-8597-4b17-97b2-aea5d6289e8b Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…ilotCliStyleSummary Agent-Logs-Url: https://github.com/github/gh-aw/sessions/e919a297-8597-4b17-97b2-aea5d6289e8b Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
✅ All tools validated successfully! Agent Container Smoke Test confirms agent container is ready. |
|
🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation... |
There was a problem hiding this comment.
Pull request overview
This PR improves the readability/consistency of action log step summaries by rendering a 2-line preview for tool output and by simplifying agent message prefixes.
Changes:
- Adds a shared
formatResultPreview()helper to render the first 2 non-empty tool output lines with tree-branch formatting and a “(+ N more)” suffix. - Updates agent message formatting to use a single
◆prefix for the first line and indentation for continuation lines. - Fixes conversation line counting to account for multi-line tool preview strings.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/log_parser_shared.cjs | Implements formatResultPreview(), updates agent message formatting, and applies 2-line tool previews + corrected line counting in both summary generators. |
| actions/setup/js/log_parser_shared.test.cjs | Updates summary expectations for the new agent prefix and 2-line tool preview behavior. |
| actions/setup/js/log_parser_bootstrap.test.cjs | Updates bootstrap test expectation for the new agent prefix in generated summaries. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 3
| */ | ||
| function formatResultPreview(resultText, maxLineLength = 80) { | ||
| if (!resultText) return ""; | ||
| const resultLines = resultText.split("\n").filter(l => l.trim()); |
There was a problem hiding this comment.
formatResultPreview() splits only on "\n" and keeps the original line strings, so Windows-style CRLF output will leave trailing "\r" characters in the preview (can render as stray ^M or break alignment). Consider normalizing line endings (e.g., split on /\r?\n/ and/or strip a trailing \r from each line) before building the preview.
| const resultLines = resultText.split("\n").filter(l => l.trim()); | |
| const resultLines = resultText.split(/\r?\n/).filter(l => l.trim()); |
There was a problem hiding this comment.
Good catch! Normalizing CRLF line endings with /\r?\n/ is a solid defensive move for Windows-generated tool outputs.
📰 BREAKING: Report filed by Smoke Copilot · ● 697.8K
| const resultLines = resultText.split("\n").filter(l => l.trim()); | ||
| if (resultLines.length === 0) return ""; | ||
|
|
||
| const line1 = resultLines[0].substring(0, maxLineLength); | ||
| if (resultLines.length === 1) { | ||
| return ` └ ${line1}`; | ||
| } | ||
|
|
||
| const line2 = resultLines[1].substring(0, maxLineLength); | ||
| if (resultLines.length === 2) { | ||
| return ` ├ ${line1}\n └ ${line2}`; | ||
| } | ||
|
|
||
| return ` ├ ${line1}\n └ ${line2} (+ ${resultLines.length - 2} more)`; |
There was a problem hiding this comment.
formatResultPreview() uses resultText.split("\n").filter(...), which allocates an array of all lines even though the summary only needs the first two non-empty lines and a count. For very large tool outputs this can create avoidable memory pressure; consider scanning the string line-by-line and collecting the first 2 non-empty lines while counting the remainder without building a full array.
| const resultLines = resultText.split("\n").filter(l => l.trim()); | |
| if (resultLines.length === 0) return ""; | |
| const line1 = resultLines[0].substring(0, maxLineLength); | |
| if (resultLines.length === 1) { | |
| return ` └ ${line1}`; | |
| } | |
| const line2 = resultLines[1].substring(0, maxLineLength); | |
| if (resultLines.length === 2) { | |
| return ` ├ ${line1}\n └ ${line2}`; | |
| } | |
| return ` ├ ${line1}\n └ ${line2} (+ ${resultLines.length - 2} more)`; | |
| let firstLine = ""; | |
| let secondLine = ""; | |
| let nonEmptyLineCount = 0; | |
| let start = 0; | |
| while (start <= resultText.length) { | |
| const newlineIndex = resultText.indexOf("\n", start); | |
| const end = newlineIndex === -1 ? resultText.length : newlineIndex; | |
| const line = resultText.substring(start, end); | |
| if (line.trim()) { | |
| nonEmptyLineCount += 1; | |
| if (nonEmptyLineCount === 1) { | |
| firstLine = line.substring(0, maxLineLength); | |
| } else if (nonEmptyLineCount === 2) { | |
| secondLine = line.substring(0, maxLineLength); | |
| } | |
| } | |
| if (newlineIndex === -1) { | |
| break; | |
| } | |
| start = newlineIndex + 1; | |
| } | |
| if (nonEmptyLineCount === 0) return ""; | |
| if (nonEmptyLineCount === 1) { | |
| return ` └ ${firstLine}`; | |
| } | |
| if (nonEmptyLineCount === 2) { | |
| return ` ├ ${firstLine}\n └ ${secondLine}`; | |
| } | |
| return ` ├ ${firstLine}\n └ ${secondLine} (+ ${nonEmptyLineCount - 2} more)`; |
| const line1 = resultLines[0].substring(0, maxLineLength); | ||
| if (resultLines.length === 1) { | ||
| return ` └ ${line1}`; | ||
| } | ||
|
|
||
| const line2 = resultLines[1].substring(0, maxLineLength); | ||
| if (resultLines.length === 2) { |
There was a problem hiding this comment.
Preview lines are truncated with substring(0, maxLineLength) but no ellipsis/indicator is added when truncation occurs. Previously some tool previews appended "..." when exceeding 80 chars; adding a truncation marker would make cut-offs explicit and avoid confusing partial values in the summary.
Agent Container Tool Check
Result: 12/12 tools available ✅ Overall Status: PASS
|
|
✅ Smoke test 23985538357 — Copilot engine\n\n| Test | ||\n|---|---|\n| GitHub MCP | ✅ |\n| MCP Scripts GH CLI | ✅ |\n| Playwright | ✅ |\n| Web Fetch | ✅ |\n| File Write/Read | ✅ |\n| Build | ✅ |\n| Discussion + PR Review | ✅ |\n\nOverall: PASS
|
There was a problem hiding this comment.
Clean refactor extracting formatResultPreview() and upgrading agent message formatting. The ◆ prefix and tree-branch style (├/└) are a nice touch for readability. Bug fix for multi-line conversationLineCount is correct.
📰 BREAKING: Report filed by Smoke Copilot · ● 697.8K
| */ | ||
| function formatResultPreview(resultText, maxLineLength = 80) { | ||
| if (!resultText) return ""; | ||
| const resultLines = resultText.split("\n").filter(l => l.trim()); |
There was a problem hiding this comment.
The new formatResultPreview function looks clean! The ◆ prefix for agent messages and tree-branch style (├/└) for tool results is a nice visual improvement over the old Agent: prefix. Good use of the extracted helper to reduce duplication across 3 call sites.
| if (resultPreview) { | ||
| lines.push(resultPreview); | ||
| conversationLineCount++; | ||
| conversationLineCount += resultPreview.split("\n").length; |
There was a problem hiding this comment.
The conversationLineCount += resultPreview.split("\n").length fix is correct — previously only 1 line was counted even for multi-line previews, which could cause premature truncation of the conversation log.
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
@copilot review comments |
Smoke Test Run 23985538383 ResultsCore Tests #1-12: PR Review Tests #13-19: Overall: PARTIAL (Tavily 429)
|
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
💥 [THE END] — Illustrated by Smoke Claude · ● 317.3K
| * @param {number} [maxLineLength=80] - Maximum characters per preview line | ||
| * @returns {string} Formatted preview string, or empty string if no content | ||
| */ | ||
| function formatResultPreview(resultText, maxLineLength = 80) { |
There was a problem hiding this comment.
Nice addition of formatResultPreview as a reusable helper! Extracting this into its own function makes it much cleaner than the duplicated inline logic that existed before. The tree-branch characters (├, └) give a nice visual hierarchy consistent with copilot-cli style.
| @@ -1742,9 +1742,9 @@ describe("log_parser_shared.cjs", () => { | |||
| const result = generatePlainTextSummary(logEntries, { parserName: "Agent" }); | |||
|
|
|||
| expect(result).toContain("Conversation:"); | |||
There was a problem hiding this comment.
The updated test expectations correctly reflect the new ◆ prefix for agent messages instead of the old Agent: prefix. Good to see multi-line agent responses are tested here with the (two-space indent) for continuation lines.
…ning, truncation ellipsis - Normalize Windows CRLF line endings by stripping trailing \r from each line - Replace full array allocation with line-by-line scanning to avoid memory pressure for large tool outputs (only first 2 non-empty lines needed) - Add "..." ellipsis when a preview line is truncated at maxLineLength - Add 8 unit tests covering all new behaviours Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b80da586-4cc8-45c6-9e54-a9d5876ffa93 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
All three review comments addressed in c99c027:
8 new unit tests cover all three behaviours (empty/whitespace input, CRLF endings, exact-length vs truncated lines, blank line skipping). |
The step summary rendered only a single truncated line (or "X lines...") for tool output, and agent messages used a repetitive
Agent:prefix on every line.Changes
formatResultPreview()helper — new shared function that renders the first 2 non-empty lines of tool output using tree-branch characters:Single-line results still use
└; exactly 2 lines use├+└; 3+ lines show(+ N more).Agent message formatting — replaced per-line
Agent: textwith◆prefix on the first line andindent for continuation lines:Applied to both
generateCopilotCliStyleSummary(step summary) andgeneratePlainTextSummary(console/core.info) for consistency.Fixed
conversationLineCountto count actual lines in multi-line preview strings rather than always incrementing by 1.✨ PR Review Safe Output Test - Run 23985538383